-
Notifications
You must be signed in to change notification settings - Fork 43
Fix broken scatter/fill_between in log-scale axis #68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
I did try to fix this myself, but failed. So I asked Codex (gpt5-high) and it found the issue and proposed this fix, which I verified works, and I streamlined its code a bit. Here's what it had to say: > - `mplexporter/mplexporter/exporter.py:6-275` now imports `numpy/mpath` and routes collections through `_prepare_collection_data`, which preserves data-space vertices/offsets whenever a non-affine `ax.transData` branch is > involved (log scales, semilog, etc.). As a result, `fill_between`, `scatter`, and other `PathCollections` on log axes export with `pathcoordinates/offsetcoordinates == 'data'`, so the rendered polygons obey zooming/panning. > - `mpld3/tests/test_elements.py:76-103` adds regression tests for both `fill_between` and `scatter` on logarithmic axes to guard against future regressions in the exported coordinate system.
|
PS: this is the test it's talking about adding to mpld3 repo. I would open a PR there which also moves the submodule, if you agree this is meaningful: |
|
I think this might close multiple issues in mpld3 proper, but I am not going to test each of them, of course: |
|
Thanks for this! This looks good at first sight, but I need to do some more in-depth review, because (1) AI, (2) I want to make sure I actually understand the change well, and (3) the code currently is named to coincide with the original matplotlib function and argument names, which isn't necessary. Will get back to you. 😊 |
PS: I will have a couple more such AI-assisted fixes coming in the near future as I go through missing features for my use-case. All of them are open issues, and I'll always disclose when I use AI. |
|
I think this is so cool! AI-assisted coding could be a really good match for this project. I did so (AI-assisted) review of this PR and I can confirm that it works. You can find my notes here. |
|
Just checking in to say:
|
I did try to fix this myself, but failed. So I asked Codex (gpt5-high) and it found the issue and proposed this fix, which I verified works, and I streamlined its code a bit.
Here's what it had to say:
Here's an example of my app before this fix:

And after this fix:
